-
Notifications
You must be signed in to change notification settings - Fork 32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove Turing integration tests #733
Conversation
ddf6ff9
to
27637ff
Compare
Pull Request Test Coverage Report for Build 12302036826Details
💛 - Coveralls |
556cd56
to
696bb3d
Compare
e06591b
to
1055a10
Compare
21889d7
to
0d7b2d1
Compare
0d7b2d1
to
675b40f
Compare
d023c04
to
543a97b
Compare
543a97b
to
7bfd233
Compare
…-> demo_assume_multivariate_observe_literal
7bfd233
to
7a93e4b
Compare
7a93e4b
to
eafcf3c
Compare
What do you think is the right order to merge this and TuringLang/Turing.jl#2419? |
Co-authored-by: Markus Hauru <[email protected]>
This one first, as it's not just tests being modified but also source code so the Turing PR will have to handle the breaking changes here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one first, as it's not just tests being modified but also source code so the Turing PR will have to handle the breaking changes here.
Sounds good. Given that this creates a moment when the tests are in neither package, we should make sure to readd them in Turing.jl soon.
Great stuff, thanks @penelopeysm!
Closes #703
General breaking changes
link!
andinvlink!
. They've been deprecated for about two years now: https://github.com/TuringLang/DynamicPPL.jl/blame/f0c31f045ccd9b69374a68ea24d3823d632a5234/src/varinfo.jl#L1323-L1329Breaking changes in
TestUtils.DEMO_MODELS
demo_assume_observe_literal
has been renamed todemo_assume_multivariate_observe_literal
to avoid name clash with new modeldemo_assume_literal_dot_observe
renamed todemo_assume_dot_observe_literal
to match the naming pattern of the other modelsdemo_assume_observe_literal
, which is a univariate assumeThis is a slightly annoying change, because anybody who uses
demo_assume_observe_literal
will have the behaviour changed silently. But the only code that uses it is DynamicPPL and Turing, so I think it's fine, as long as I fix it upstream too. (See GitHub-wide search fordemo_assume_observe_literal
)Changes to test-internal utils
make_chain_from_prior([rng,] model, n_iters) which constructs an MCMCChains.Chains object by sampling from the prior of
modelfor
n_iters` iterations.Note this is in
test/test_util.jl
rather thansrc/test_utils/foo.jl
, hence only accessible inside the test suite.Changes to test suite
make_chain_from_prior
function above), or some sampling algorithm with somespace
parameter (which has been replaced with a dummy algorithmMyAlg
intest/varinfo.jl
).Feedback needed:
make_chain_from_prior
seems too useful to languish intest/test_util.jl
. Should this be in DynamicPPLMCMCChainsExt?